feat: support option for continuous monitoring token usage in streaming response #111
Conversation
| // MonitorContinuousUsageStats flag controls if external process monitors every response-body chunk for usage stats | ||
| // when true, it will monitor for token metadata usage in every response-body chunk received during request in streaming mode | ||
| // compatible with vllm's 'continuous_usage_stats' flag | ||
| // when false, it will stop monitoring after detecting token metadata usage after finding it for the first time. | ||
| // compatible with OpenAI's streaming response (https://platform.openai.com/docs/api-reference/chat/streaming#chat/streaming-usage) | ||
| // Only affects request in streaming mode | ||
| MonitorContinuousUsageStats bool `yaml:"monitorContinuousUsageStats,omitempty"` |
There was a problem hiding this comment.
could you remove the change related to this? I think this is another issue and metadata is not cumulative so basically it's overriding previous ones if it's emitted in the middle.
There was a problem hiding this comment.
I think this should be a property on the AIServiceBackend as only certain backend supports this, e.g vLLM service backend.
|
sorry this feels a conflict with #103 i would appreciate it if you could stop it until it lands - I think the PR will supersede this PR besides vllm stuff part |
|
@sukumargaonkar thank you for waiting - #103 has landed so could you rework the PR and focus on the vllm stuff? |
|
ping |
|
@sukumargaonkar do you still want continue the PR here? I will close this in a few days if there's no response as there's no reason to keep it open |
|
yes, will rebase and include only the vllm specific changes |
|
great! |
|
checking - how long do you need to rework the pr here? @sukumargaonkar it should be pretty straightforward right? i wonder if this can get in the initial release. |
Currently only total_tokens usage from response body are pushed to dynamicMetadata. This PR updates that logic to include input and output token usage as well. Signed-off-by: Sukumar Gaonkar <[email protected]>
✅ Deploy Preview for envoy-ai-gateway canceled.
|
yuzisun
changed the title
I made a comment above, it should be a flag for |
|
@yuzisun I think i should've rephrased the question: why don't we just enable this logic by default (== removing the .bufferingDone flag)? In anyways, we parse all event by default (since the used token event is always the last one), so this option doesn't help us at all in terms of the computation. (On that note, i think i shouldn't have introduced |
|
i agree, its better to parse every msg chunk while streaming to check for presence of usage data i don't think we need to do the aggregation here ai-gateway/internal/extproc/processor.go Lines 203 to 205 in 2c62874 thoughts @mathetake ? |
i see that's a good finding. is there any documentation about that? then let's change the aggregation as well as removing the |
|
ping |
|
@sukumargaonkar hey are you still interested in this ? I would prefer the fast turnaround of a single PR and avoid keeping the context for a long time, so i would appreciate if you could rework the PR soon. Otherwise, i will close and redo it by myself. Thanks ! |
Currently only total_tokens usage from response body are pushed to dynamicMetadata. This PR updates that logic to include input and output token usage as well.
This PR also introduces
monitorContinuousUsageStatsflag in config for external processThe flag controls if external process monitors every response-body chunk for usage stats
when true, it will monitor for token metadata usage in every response-body chunk received during request in streaming mode (compatible with vllm's 'continuous_usage_stats' flag)
when false, it will stop monitoring after detecting token metadata usage after finding it for the first time.
(compatible with OpenAI's streaming response (https://platform.openai.com/docs/api-reference/chat/streaming#chat/streaming-usage))
Only affects request in streaming mode